Skip to content

Fixing the demo to not provoque ctrl actions with modded keyboards or mobile virtual keyboards#879

Open
belug23 wants to merge 3 commits into
Immediate-Mode-UI:masterfrom
belug23:sdl3_demo_fix_ctrl_actions
Open

Fixing the demo to not provoque ctrl actions with modded keyboards or mobile virtual keyboards#879
belug23 wants to merge 3 commits into
Immediate-Mode-UI:masterfrom
belug23:sdl3_demo_fix_ctrl_actions

Conversation

@belug23

@belug23 belug23 commented Jan 29, 2026

Copy link
Copy Markdown

While using a modded keyboard with macros on the characters pressing the keys affected by the ctrl modifier without ctrl down, would still trigger it, it was the same issue using the Android virtual keyboard.

Here's an example of what was happening, pressing a would erase all text and write the letter a instead, pressing b would write the letter b at the start of the text area and set the cursor in between the first and second character, pressing e would add the letter e at the end of the text area.

After some code investigation, I found out that calling nk_input_key would always increase ctx->input.keyboard.keys[key].clicked even if ctrl wasn't push, then if it was released while the clicked counter was still equals to 1, it would increase it to 2 triggering the nk_input_key_pressed due to the validation of if ((k->down && k->clicked) || (!k->down && k->clicked >= 2)).

Only calling nk_input_key when ctrl is down fixes the problem.

sleeptightAnsiC

This comment was marked as outdated.

@sleeptightAnsiC

sleeptightAnsiC commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

@belug23 Can you open a separate Issue for this? I do agree we have a bug here, just not with the proposed fix.

It just "happens to work" with this patch, but it looks like the library and the backend are both fighting over who should be handling this, and in the long run it will most likely break again.

@sleeptightAnsiC

This comment was marked as outdated.

@belug23

belug23 commented Jan 31, 2026

Copy link
Copy Markdown
Author

As discussed in #882 I've updated the code to handle the release of the special functions.

@sleeptightAnsiC sleeptightAnsiC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just note that I still don't consider this to be exactly correct, and it should NOT be merged yet. There are many nuances mentioned about this issue in #882 We decided to update this PR in order to have something to work with.

@RobLoach RobLoach left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. This logic always catches me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants